Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make code slightly more Pythonic #16

Closed
wants to merge 19 commits into from
Closed

Make code slightly more Pythonic #16

wants to merge 19 commits into from

Conversation

msoucy
Copy link
Contributor

@msoucy msoucy commented May 23, 2014

This doesn't make EVERYTHING more pythonic, but it's a huge step forward - mostly taking advantage of Python builtins and syntax, with some formatting.

@msoucy msoucy changed the title Refactor/pythonic Make code slightly more Pythonic May 23, 2014
@msoucy
Copy link
Contributor Author

msoucy commented May 23, 2014

I apologize for the size of the request, I started hacking on it and time just flew by.

@polaris-
Copy link
Collaborator

Woah. That's a lot of changes. Github won't let me automatically merge everything so it might take me a little bit to go through them all, but I'll try to get them all before tonight or tomorrow. Thanks a lot! Neither Toad King or I are really Python programmers so something like this was a long time coming.

@msoucy
Copy link
Contributor Author

msoucy commented May 23, 2014

Yeah, sorry about that - if you want to wait until tonight I can attempt to merge master into my branch, to make it easier.

Yeah, I was a bit curious about the choice to use Python when your code style was (somewhat obviously, if you don't mind me saying) very C-like.

Most of these commits are related to doing things using builtin members and syntax instead of manually recreating them.

I've planned another change for gs_query that will greatly shrink the code down and make it nicer, assuming that what I believe about the query strings is correct (-separated key-value pairs, separated with , the "final" key doesn't have a value because it ends the list for that query). I also REALLY want to clean up the parser you're manually implementing in gamespy_backend_server, I feel that it can be made much simpler than using string indices directly.

Unfortunately, this pull request won't put everything into a more pythonic style, but it's a start.

@polaris-
Copy link
Collaborator

Alright, that sounds fine. I'll wait until you do that then.

I mostly programming in C, C++, and C#, so that influence is obviously going to come out. This was my first real Python project outside of a class I took in college. I originally chose Python so I could just quickly get something together without needing to always recompile, but I became too lazy to rewrite it in another language, so it stuck.

If you feel like you can improve those parts then by all means please do. If the functionality is the same then I would be more than happy to have cleaner code. If you wouldn't mind, please make sure to put some comments for what might be more the more trickier parts of the code so I will know what's going on and be able to learn from it.

@msoucy
Copy link
Contributor Author

msoucy commented May 24, 2014

Due to some time constraints, I didn't have time to do the (admittedly sizeable) merge. I think it might be better to close this, and make a bunch of smaller pull requests over the next couple of days, each fixing one specific thing. That way there's less trouble merging all the fixes

@polaris-
Copy link
Collaborator

Alright, no problem. Take your time. I will close this pull request and wait for your smaller pull requests then. Thanks.

@polaris- polaris- closed this May 24, 2014
@msoucy msoucy deleted the refactor/pythonic branch May 26, 2014 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants